Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fork Jasmine #3147

Merged
merged 7 commits into from
Mar 16, 2017
Merged

Fork Jasmine #3147

merged 7 commits into from
Mar 16, 2017

Conversation

cpojer
Copy link
Member

@cpojer cpojer commented Mar 15, 2017

Summary

This diff removes half of Jasmine's implementation, cleans up the project by pretty-printing it and fixes all lint errors. For the longest time we've had our own fork of Jasmine. Updating Jasmine, even though it is still maintained, doesn't really provide us any value. It is only a tiny part of the Jest testing platform and we aren't using many features of it. Our plan has been to replace Jasmine but realistically the only way this is going to happen is if we rewrite our test-runner in place without breaking changes. This diff is not the first step: over the last year we have incrementally been doing exactly this. We built our own assertion library, asymmetric matchers and spies. This is simply the next step. The remaining pieces of Jasmine are now basically only the actual spec runner and the reporter API. Once we have async reporters (cc @DmitriiAbramov) we can remove Jasmine's reporters entirely and build an adapter API.

Changes:

  • Changed how Jasmine is initialized and exported, removes the weird indirection.
  • Fixed asymmetric matcher aliases to actually be used from Jest. They were in the wrong place.
  • Upgraded code style

Removed Features:

  • Matchers
  • Asymmetric Matchers
  • MockDate
  • "Order" to randomize tests
  • util module
  • matcher utils
  • pretty-printer
  • Jasmine globals that are unnecessary
  • A clone function that has been broken for 6 months and hasn't been called.

Test plan

yarn test

Next Steps (volunteers welcome)

  • Build an adapter for Jasmine spies using jest-mock (jest.spyOn etc.) and remove Jasmine's spy system.
  • Identify further things that can be dropped.
  • Completely rewrite how errors are handled and turn them into a structured format (cc @kentaromiura)
  • Tomorrow I will create an issue about how we can build our own test runner. We'll need an RFC with the new APIs and an incremental path forward.

diaf

@jest-bot
Copy link
Contributor

jest-bot commented Mar 15, 2017

Fails
🚫 New JS files do not have the Facebook copyright header: packages/jest-jasmine2/src/jasmine-light.js
Warnings
⚠️ Please ensure that @flow is enabled on: packages/jest-jasmine2/src/jasmine-light.js
⚠️ Changes were made to package.json, but not to yarn.lock - Perhaps you need to run `yarn install`?

Generated by 🚫 dangerJS

Copy link
Contributor

@aaronabramov aaronabramov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes me happy!

displayErrors: true,
filename: JASMINE_PATH,
});
const JASMINE_PATH = require.resolve('./jasmine-2.5.2.js');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think at this point we can just call it jasmine and strip 2.5.2

};

exports.version = function() {
return '2.5.2-custom';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'jasmine-light'? :)

@cpojer cpojer merged commit e79ad74 into jestjs:master Mar 16, 2017
@cpojer
Copy link
Member Author

cpojer commented Mar 16, 2017

Let's do it then.

skovhus pushed a commit to skovhus/jest that referenced this pull request Apr 29, 2017
* Remove unused and unsupported Jasmine features.

* Move jasmine to src.

* Jasmine is now a beautiful piece of pretty printed code.

* Fix up stack trace filtering.

* Lint fixes.

* More dead code removal.

* Call it jasmine-light.
@cpojer cpojer deleted the kill-jasmine branch May 4, 2017 15:44
tushardhole pushed a commit to tushardhole/jest that referenced this pull request Aug 21, 2017
* Remove unused and unsupported Jasmine features.

* Move jasmine to src.

* Jasmine is now a beautiful piece of pretty printed code.

* Fix up stack trace filtering.

* Lint fixes.

* More dead code removal.

* Call it jasmine-light.
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants